Skip to content

feat(2fa): profile updates and 2fa#128

Merged
gaboesquivel merged 4 commits intomainfrom
2fa
Mar 9, 2026
Merged

feat(2fa): profile updates and 2fa#128
gaboesquivel merged 4 commits intomainfrom
2fa

Conversation

@gaboesquivel
Copy link
Copy Markdown
Member

@gaboesquivel gaboesquivel commented Mar 9, 2026

Summary by CodeRabbit

  • Refactor

    • Authentication gating centralized to the dashboard layout; per-page auth redirects removed so pages render without automatic redirect checks.
  • New Features

    • New reusable dashboard shell with persistent sidebar, responsive header (health/auth indicators), assistant panel, toast notifications, and a sign-out flow.
  • Chores

    • Strengthened JWT verification and server secret handling; added auth-related query keys and build/script adjustments for JWT availability.
  • Documentation

    • Added guidance clarifying route protection and avoiding duplicate auth redirects.

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
basilic-docs Ready Ready Preview, Comment Mar 9, 2026 6:33pm
basilic-fastify Ready Ready Preview, Comment Mar 9, 2026 6:33pm
basilic-next Ready Ready Preview, Comment Mar 9, 2026 6:33pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 9, 2026

Warning

Rate limit exceeded

@gaboesquivel has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 18 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9e58dfd0-1e42-4da7-bf66-0f9b3992c09f

📥 Commits

Reviewing files that changed from the base of the PR and between daa654f and 16d0618.

📒 Files selected for processing (3)
  • apps/next/app/(dashboard)/_dashboard-shell.tsx
  • apps/next/lib/auth/jwt-utils.ts
  • apps/next/proxy.ts

Walkthrough

Centralizes auth verification (JWT_SECRET + verifyJwtToken) to the server/proxy, removes page-level auth redirects, adds a client-side DashboardShell with sign-out and UI panels, and propagates JWT_SECRET through env, build scripts, and query-key utilities.

Changes

Cohort / File(s) Summary
Dashboard pages (auth removal)
apps/next/app/(dashboard)/(news)/page.tsx, apps/next/app/(dashboard)/markets/page.tsx
Removed page-level getAuthStatus + redirect logic; pages now fetch and render data without performing auth redirects.
Dashboard layout & shell
apps/next/app/(dashboard)/layout.tsx, apps/next/app/(dashboard)/_dashboard-shell.tsx
Replaced prior dashboard layout with async Layout that renders the new client-side DashboardShell; added DashboardShell component (sidebar, header, ApiHealthBadge/AuthBadge, sign-out flow, AssistantSidebar, Toaster).
JWT verification & auth utils
apps/next/lib/auth/jwt-utils.ts, apps/next/lib/auth/auth-utils.ts, apps/next/proxy.ts
Added verifyJwtToken (uses jwtVerify) and switched to async verification with env.JWT_SECRET; updated isTokenExpired to parse payload manually when needed; replaced decodeJwtToken usages with verification in auth paths.
Environment and build/test config
apps/next/lib/env.ts, turbo.json, apps/next/scripts/run-e2e-local.mjs, scripts/run-qa.mjs
Introduced JWT_SECRET into server env schema and runtimeEnv; updated build/e2e/qa scripts and turbo env lists to provide/forward a JWT_SECRET fallback for builds/tests.
Query keys & infra
apps/next/lib/query-keys.ts, .gitleaks.toml
Added auth session query key constants (['auth','session','user'], ['auth','session','jwt']) and added scripts/run-qa\.mjs$ to gitleaks allowlist.
Docs / guidance
.cursor/rules/frontend/auth.mdc, apps/docu/content/docs/architecture/authentication.mdx
Documented that proxy.ts is the single source for route protection and advised against page/layout-level redirect guards.

Sequence Diagram(s)

sequenceDiagram
  participant Browser as Browser (Client)
  participant NextServer as NextJS Server / proxy
  participant DashboardShell as Client (DashboardShell)
  participant API as Auth API (/auth/logout)
  participant QueryClient as react-query Cache

  Browser->>NextServer: GET /dashboard
  NextServer->>NextServer: verifyJwtToken(token, env.JWT_SECRET)
  alt unauthenticated
    NextServer-->>Browser: Redirect /auth/login
  else authenticated
    NextServer-->>Browser: Serve dashboard HTML (Layout + DashboardShell)
    Browser->>DashboardShell: Hydrate/render shell (sidebar, header, badges)
  end

  note over Browser,DashboardShell: User triggers sign-out
  Browser->>API: POST /auth/logout
  API-->>Browser: 200 OK
  Browser->>QueryClient: Invalidate `['auth','session','user']` and `['auth','session','jwt']`
  Browser-->>Browser: Redirect to /
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I hopped through tokens, shell, and door,
Server keeps the keys, pages guard no more.
A sidebar hug, a logout little thump,
Cookies checked, then off I jump —
Carrots await at the homepage shore!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title 'feat(2fa): profile updates and 2fa' does not match the actual changes, which focus on removing authentication checks from pages, centralizing auth via proxy.ts, and restructuring the dashboard layout. Update the title to reflect the main changes, such as 'feat: centralize auth routing via proxy and refactor dashboard layout' or similar.
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2fa

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gaboesquivel gaboesquivel changed the title refactor(next): extract dashboard shell and centralize auth in layout feat(2fa): profile updates and 2fa Mar 9, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
apps/next/app/(dashboard)/layout.tsx (1)

6-6: Add an explicit return type to the exported layout.

Please annotate Layout with an explicit return type, e.g. Promise<React.JSX.Element>, to keep exported functions explicit.

Proposed change
-export default async function Layout({ children }: Readonly<{ children: React.ReactNode }>) {
+export default async function Layout(
+  { children }: Readonly<{ children: React.ReactNode }>,
+): Promise<React.JSX.Element> {

As per coding guidelines, "TypeScript practices: strict mode, no any, explicit return types for exported functions, type-only imports, and colocated schemas".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/next/app/`(dashboard)/layout.tsx at line 6, The exported async function
Layout lacks an explicit return type; update its signature (the Layout function)
to include an explicit return type such as Promise<React.JSX.Element> (or the
appropriate JSX element type used in the app) so the declaration reads like:
export default async function Layout(...): Promise<React.JSX.Element> { ... };
this satisfies the project's TypeScript guideline for explicit return types on
exported functions.
apps/next/app/(dashboard)/_dashboard-shell.tsx (1)

15-15: Add an explicit return type to the exported shell.

Please annotate DashboardShell with an explicit return type, e.g. React.JSX.Element, so the public surface stays typed consistently.

Proposed change
-export function DashboardShell({ children }: Readonly<{ children: React.ReactNode }>) {
+export function DashboardShell(
+  { children }: Readonly<{ children: React.ReactNode }>,
+): React.JSX.Element {

As per coding guidelines, "TypeScript practices: strict mode, no any, explicit return types for exported functions, type-only imports, and colocated schemas".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/next/app/`(dashboard)/_dashboard-shell.tsx at line 15, Add an explicit
return type to the exported component function by annotating DashboardShell's
signature with React.JSX.Element (e.g. change "export function DashboardShell({
children }: Readonly<{ children: React.ReactNode }>)" to "export function
DashboardShell({ children }: Readonly<{ children: React.ReactNode }>):
React.JSX.Element" ); ensure any needed type-only import for React is used if
your tsconfig requires it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/next/app/`(dashboard)/_dashboard-shell.tsx:
- Around line 29-30: The AuthBadge import in _dashboard-shell.tsx points to the
wrong package; update the import to the local component at
apps/next/components/shared/auth-badge.tsx (the AuthBadge component that uses
useUser()). Also modify the logout route handler (route.ts in app/auth/logout)
to explicitly invalidate the auth/session/user query cache after clearing
cookies by calling queryClient.invalidateQueries({ queryKey:
['auth','session','user'] })—follow the same pattern used in use-passkey-auth.ts
and use-link-email.ts so the badge updates immediately after logout.

In `@apps/next/app/`(dashboard)/layout.tsx:
- Around line 7-8: Layout is missing an explicit return type and getAuthStatus
performs decode-only JWT checks allowing forged tokens; add a return type (e.g.,
React.ReactNode or JSX.Element) to the Layout function signature and replace the
decode-only validation in getAuthStatus with proper signature verification using
jose's verifyJwt (or call a server action that runs verifyJwt on the token
server-side) so the cookie/JWT is cryptographically validated before rendering;
also update auth-server cookie creation to set httpOnly: true so the token is
not accessible to client-side JS.

---

Nitpick comments:
In `@apps/next/app/`(dashboard)/_dashboard-shell.tsx:
- Line 15: Add an explicit return type to the exported component function by
annotating DashboardShell's signature with React.JSX.Element (e.g. change
"export function DashboardShell({ children }: Readonly<{ children:
React.ReactNode }>)" to "export function DashboardShell({ children }: Readonly<{
children: React.ReactNode }>): React.JSX.Element" ); ensure any needed type-only
import for React is used if your tsconfig requires it.

In `@apps/next/app/`(dashboard)/layout.tsx:
- Line 6: The exported async function Layout lacks an explicit return type;
update its signature (the Layout function) to include an explicit return type
such as Promise<React.JSX.Element> (or the appropriate JSX element type used in
the app) so the declaration reads like: export default async function
Layout(...): Promise<React.JSX.Element> { ... }; this satisfies the project's
TypeScript guideline for explicit return types on exported functions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f069a509-1ab2-49e8-8b52-9638c6822cea

📥 Commits

Reviewing files that changed from the base of the PR and between a3b11d0 and d856ac0.

📒 Files selected for processing (4)
  • apps/next/app/(dashboard)/(news)/page.tsx
  • apps/next/app/(dashboard)/_dashboard-shell.tsx
  • apps/next/app/(dashboard)/layout.tsx
  • apps/next/app/(dashboard)/markets/page.tsx
💤 Files with no reviewable changes (2)
  • apps/next/app/(dashboard)/(news)/page.tsx
  • apps/next/app/(dashboard)/markets/page.tsx

… return types

- Replace decode-only JWT check with jose verifyJwt (crypto validation)
- Add JWT_SECRET to Next env (server, prod-required)
- Client-side logout with invalidateQueries for auth/session/user and jwt
- Add explicit return types to Layout and DashboardShell
- Add query-keys.ts for ESLint compliance, pass JWT_SECRET to e2e build
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
apps/next/lib/auth/auth-utils.ts (1)

29-33: Remove redundant expiration check.

verifyJwtToken() already validates the exp claim via jwtVerify() (which throws JWTExpired if expired). The subsequent isTokenExpired() call on line 33 re-parses the raw token payload using identical logic (exp * 1000 <= Date.now()), making it redundant. Since no clock tolerance or custom date options are configured in either function, a token that passes verification cannot be expired. Delete the isTokenExpired() check and trust the verified payload.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/next/lib/auth/auth-utils.ts` around lines 29 - 33, The code redundantly
re-checks token expiration after verification; remove the call to
isTokenExpired({ token }) and its conditional return, because verifyJwtToken({
token, secret: env.JWT_SECRET }) already enforces the exp claim (throwing on
expiry) and ensures decoded is valid; update the block around
verifyJwtToken/decoded to rely on decoded.typ, decoded.sub, decoded.sid only and
delete the isTokenExpired check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.gitleaks.toml:
- Line 31: Remove the broad allowlist entry '''scripts/run-qa\.mjs$''' in
.gitleaks.toml and instead add an allowlist rule that targets only the specific
sentinel string used as the placeholder JWT in scripts/run-qa.mjs (i.e., the
exact literal token or the constant name defined there), using a precise regex
or exact match so other secrets in that file remain detectable; update the rule
to reference that exact sentinel value rather than the whole filename.

In `@apps/next/app/`(dashboard)/_dashboard-shell.tsx:
- Around line 21-25: In handleSignOut, don't assume logout succeeded: check the
fetch('/auth/logout') response (use response.ok and/or response.status) and only
call queryClient.invalidateQueries(authSessionUserQueryKey |
authSessionJwtQueryKey) and set window.location.href='/' after a successful
response; on non-2xx responses, surface an error (throw, show UI toast, or
console.error) and avoid clearing client auth state or redirecting so the UI
remains consistent with the server session.

In `@apps/next/lib/auth/jwt-utils.ts`:
- Around line 33-39: Replace the manual base64 decode in isTokenExpired() with
the existing decodeJwtToken() helper: call decodeJwtToken(token) to get the
payload, then check payload.exp (convert to ms) to determine expiry; ensure you
handle a missing or unparsable payload by returning true (expired) and avoid
using atob() or direct token.split('.') decoding. Update references in
isTokenExpired() to use decodeJwtToken() and retain the same boolean semantics.

In `@apps/next/proxy.ts`:
- Around line 25-27: The proxy currently calls verifyJwtToken(...) (which uses
jwtVerify and enforces exp) and treats a null result as unauthenticated,
blocking the refresh path; change the flow so you first inspect the token
without enforcing expiration (either by using an existing isTokenExpired(token)
to decide refresh vs reject, or by decoding the token/verifying with expiration
ignored) to allow expired-but-refreshable tokens to reach the refresh logic,
then perform full verification (verifyJwtToken / jwtVerify) only after deciding
not to refresh; update the logic around verifyJwtToken, isTokenExpired and the
refresh decision so typ/sub/sid are checked from the decoded claims or after
ignoring exp, and only enforce exp when appropriate.

---

Nitpick comments:
In `@apps/next/lib/auth/auth-utils.ts`:
- Around line 29-33: The code redundantly re-checks token expiration after
verification; remove the call to isTokenExpired({ token }) and its conditional
return, because verifyJwtToken({ token, secret: env.JWT_SECRET }) already
enforces the exp claim (throwing on expiry) and ensures decoded is valid; update
the block around verifyJwtToken/decoded to rely on decoded.typ, decoded.sub,
decoded.sid only and delete the isTokenExpired check.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 81722bce-fc39-4775-bcf8-2a65b0a004ac

📥 Commits

Reviewing files that changed from the base of the PR and between d856ac0 and 338f09e.

⛔ Files ignored due to path filters (1)
  • apps/next/.env.production is excluded by !**/.env*
📒 Files selected for processing (11)
  • .gitleaks.toml
  • apps/next/app/(dashboard)/_dashboard-shell.tsx
  • apps/next/app/(dashboard)/layout.tsx
  • apps/next/lib/auth/auth-utils.ts
  • apps/next/lib/auth/jwt-utils.ts
  • apps/next/lib/env.ts
  • apps/next/lib/query-keys.ts
  • apps/next/proxy.ts
  • apps/next/scripts/run-e2e-local.mjs
  • scripts/run-qa.mjs
  • turbo.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/next/app/(dashboard)/layout.tsx

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
apps/next/app/(dashboard)/layout.tsx (1)

3-6: Consider removing async since no await is used.

The function is marked async but doesn't perform any asynchronous operations. While harmless, removing it simplifies the signature.

♻️ Suggested change
-export default async function Layout({
+export default function Layout({
   children,
-}: Readonly<{ children: React.ReactNode }>): Promise<React.JSX.Element> {
+}: Readonly<{ children: React.ReactNode }>): React.JSX.Element {
   return <DashboardShell>{children}</DashboardShell>
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/next/app/`(dashboard)/layout.tsx around lines 3 - 6, The Layout function
is marked async but contains no awaits; remove the async keyword and the Promise
return type to make it a synchronous component: change the signature of Layout
(currently "export default async function Layout({ children }: Readonly<{
children: React.ReactNode }>): Promise<React.JSX.Element>") to a plain
synchronous function returning React.JSX.Element (e.g., "export default function
Layout(...) : React.JSX.Element") and keep using DashboardShell to render
children.
.cursor/rules/frontend/auth.mdc (1)

3-3: Consider narrowing the glob pattern.

The glob includes packages/core/** and packages/react/**, but the "Auth Protection (proxy.ts)" section is specific to Next.js route handling. Core and react packages shouldn't contain getAuthStatus() + redirect() patterns since they don't use Next.js routing. Consider whether this rule should apply only to apps/next/**.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.cursor/rules/frontend/auth.mdc at line 3, The glob pattern is too broad and
may flag non-Next.js packages; narrow it so the "Auth Protection (proxy.ts)"
rule only scans Next.js app code. Update the rule configuration to target only
the Next.js app files (e.g., change the glob from including packages/core/** and
packages/react/** to only apps/next/**) so the rule that looks for
Next.js-specific symbols like getAuthStatus() and redirect() runs only against
Next.js routes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.cursor/rules/frontend/auth.mdc:
- Line 3: The glob pattern is too broad and may flag non-Next.js packages;
narrow it so the "Auth Protection (proxy.ts)" rule only scans Next.js app code.
Update the rule configuration to target only the Next.js app files (e.g., change
the glob from including packages/core/** and packages/react/** to only
apps/next/**) so the rule that looks for Next.js-specific symbols like
getAuthStatus() and redirect() runs only against Next.js routes.

In `@apps/next/app/`(dashboard)/layout.tsx:
- Around line 3-6: The Layout function is marked async but contains no awaits;
remove the async keyword and the Promise return type to make it a synchronous
component: change the signature of Layout (currently "export default async
function Layout({ children }: Readonly<{ children: React.ReactNode }>):
Promise<React.JSX.Element>") to a plain synchronous function returning
React.JSX.Element (e.g., "export default function Layout(...) :
React.JSX.Element") and keep using DashboardShell to render children.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fda01595-ecb3-4fc2-abb6-24c184292dec

📥 Commits

Reviewing files that changed from the base of the PR and between 338f09e and daa654f.

📒 Files selected for processing (4)
  • .cursor/rules/frontend/auth.mdc
  • apps/docu/content/docs/architecture/authentication.mdx
  • apps/next/app/(dashboard)/layout.tsx
  • apps/next/app/auth/login/page.tsx
💤 Files with no reviewable changes (1)
  • apps/next/app/auth/login/page.tsx

…esh flow

- handleSignOut: check response.ok/status before invalidating and redirecting;
  surface toast on failure
- isTokenExpired: use decodeJwtToken instead of manual base64/atob
- proxy: decode token without exp first (decodeJwtToken), allow expired tokens
  to reach refresh; verifyJwtToken only when not refreshing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant